Fix clash with Windows min/max macros (completes #52) - #57
Merged
Conversation
Fixes clash with Windows "min" and "max" macros
Follow-up to the original fix:
* erase() and the move helper still called std::min unparenthesized at
lines 408, 469 and 472, so <Windows.h> without NOMINMAX still failed
to compile for anyone using erase()/insert(). Simulating the Windows
min/max macros, main produced 9 errors and the original patch left 3;
now it compiles clean.
* The four touched lines used tabs, but .clang-format sets
UseTab: Never, so the lint job rejected them. Converted to spaces.
std::max<size_t>(1, starting_capacity) at line 310 deliberately stays
as is: an explicit template argument makes 'max' be followed by '<'
rather than '(', so the function-like macro is never invoked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
martinus
force-pushed
the
pr/52-windows-minmax
branch
from
July 27, 2026 15:42
6c7c74e to
47d2f42
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes #52 by @T-640. Their original commit is preserved as the first commit here, since [maintainer pushes to their fork are not possible from this environment]; merging this will close #52 as merged.
The problem
Including
<Windows.h>withoutNOMINMAXdefinesmin/maxas function-like macros, breakingsvector.h.What the original PR did
Parenthesized 4 sites (lines 137, 167, 320, 849) so
min/maxis not followed by(and the macro is not invoked. Correct approach, but incomplete.What this adds
1. Three missed call sites.
erase()and the move helper still used unparenthesizedstd::min, so anyone callingerase()/insert()still could not compile:svector.h:408—std::min(const_cast<T*>(to), container_end)svector.h:469—std::min(num_moves, std::distance(...))svector.h:472—std::min(source_end, target_begin)Measured by compiling a TU that defines the Windows
min/maxmacros before including the header:main2. Indentation. The four original lines used tabs;
.clang-formatsetsUseTab: Never, so thelintjob rejected them. Converted to spaces —./scripts/lint/lint-all.pynow passes.std::max<size_t>(1, starting_capacity)at line 310 is deliberately left alone: the explicit template argument makesmaxbe followed by<, so the function-like macro never fires.Verification
meson test:Ok: 2, Fail: 0(56986 assertions). Lint clean.Depends on #56 for CI to actually run green.
🤖 Generated with Claude Code